Support SVG clipboard paste#1149
Conversation
Deploying plait with
|
| Latest commit: |
ee30ded
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c9cdfec1.plait.pages.dev |
| Branch Preview URL: | https://nightt5879-svg-clipboard-pla.plait.pages.dev |
|
i using this test locally: |
Deploying plait-docs with
|
| Latest commit: |
ee30ded
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4db2a3bd.plait-docs.pages.dev |
| Branch Preview URL: | https://nightt5879-svg-clipboard-pla.plait-docs.pages.dev |
There was a problem hiding this comment.
The overall direction—turning SVG clipboard text into a File and reusing the existing image insertion path—looks sound. However, I think the DataTransfer flow should be simplified before this is merged.
The intended source precedence is reasonable and should remain explicit:
DataTransfer.files
→ Plait structured HTML
→ SVG MIME data
→ plain text
Only when DataTransfer is absent should the implementation fall back to navigator.clipboard.read().
1. Preserve the synchronous SVG representation before awaiting
The current expression makes DataTransferItem.getAsString() and dataTransfer.getData(SVG_MIME_TYPE) mutually exclusive:
const svgText = svgItem
? await getStringFromDataTransferItem(svgItem)
: dataTransfer.getData(SVG_MIME_TYPE);When an SVG string item exists, getData() is never read. If the asynchronous callback returns an empty value, a valid synchronous SVG representation can therefore be lost.
Please capture the synchronous value before crossing the asynchronous boundary, and use the item value only as a fallback. For example:
const readDataTransferItemAsString = (item: DataTransferItem): Promise<string> => {
return new Promise((resolve) => {
item.getAsString((value) => resolve(value || ''));
});
};
const readSvgText = async (dataTransfer: DataTransfer): Promise<string> => {
// Read this while the paste event's DataTransfer is still synchronously available.
const svgText = dataTransfer.getData(SVG_MIME_TYPE);
const svgItem = Array.from(dataTransfer.items || []).find(
(item) => item.kind === 'string' && item.type === SVG_MIME_TYPE
);
if (svgText.trim()) {
return svgText;
}
return svgItem ? await readDataTransferItemAsString(svgItem) : '';
};The naming also makes the asynchronous read and the parsing responsibility clearer than generic get* names.
2. Remove the navigator fallback from the DataTransfer branch
Please remove the navigator clipboard fallback from the dataTransfer branch, together with hasSvgClipboardType and getNavigatorClipboardSafely if they are no longer used.
The current paste event's DataTransfer should be authoritative. Calling navigator.clipboard.read() here:
- requires additional browser permission and secure-context support;
- can observe clipboard state that is different from the original paste event;
- adds a second data source even though the outer
dataTransfer === nullbranch already owns navigator fallback; - currently compensates for not preserving the synchronous SVG value before
await.
hasSvgClipboardType() also scans dataTransfer.items separately from getSvgClipboardData(). Avoiding repeated reads is preferable because clipboard data is event-scoped, and the helper name implies a broader check than its implementation—it only checks items, not getData(), types, or files.
With the fallback removed, the main flow can stay small and explicit:
export const getClipboardData = async (
dataTransfer: DataTransfer | null
): Promise<ClipboardData | null> => {
if (!dataTransfer) {
return getProbablySupportsClipboardRead()
? await getNavigatorClipboard()
: null;
}
if (dataTransfer.files.length) {
return { files: Array.from(dataTransfer.files) };
}
const plaitClipboardData = getDataTransferClipboard(dataTransfer);
if (Object.keys(plaitClipboardData).length > 0) {
return plaitClipboardData;
}
const svgText = await readSvgText(dataTransfer);
if (svgText.trim()) {
return {
files: [new File([svgText], 'plait-svg-image.svg', { type: SVG_MIME_TYPE })]
};
}
return getDataTransferClipboardText(dataTransfer);
};3. Navigator clipboard SVG handling
getNavigatorClipboard() already treats image/svg+xml as an image through its generic image/* path, calls getType('image/svg+xml'), and preserves the Blob MIME type when creating the File. It therefore does not need SVG-specific conversion for basic SVG-only clipboard support.
One separate product decision is representation priority: a ClipboardItem may expose both image/png and image/svg+xml, while downstream consumers currently use only files[0]. If SVG must be preferred to preserve vector data, please make that an explicit requirement and add an ordering test; otherwise no navigator-specific SVG change is needed in this PR.
Required regression test
Please add a test where:
- an SVG string item exists;
getAsString()returns an empty value;getData('image/svg+xml')contains valid SVG;- navigator clipboard access is unavailable.
The expected result should still be an SVG File created from the synchronous DataTransfer value.
|
Updated, thanks. The DataTransfer path now reads the synchronous SVG value before |
Summary
image/svg+xmlclipboard entries and convert them into image files for the existing paste flow.getData, and navigator clipboard fallback paths.Testing
npx ng test core --watch=false --progress=false --browsers=ChromeHeadlessCI --include=packages/core/src/utils/clipboard/clipboard.spec.tsnpm run build:corenpm run build:common